-
Notifications
You must be signed in to change notification settings - Fork 13
Morse decoder #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Morse decoder #49
Conversation
WalkthroughThis update introduces two new EOG-based applications: a double/triple blink detector and a Morse code decoder, both leveraging LSL data streams and real-time signal processing. The configuration is updated to register the Morse Decoder app. The README is enhanced with improved setup instructions, troubleshooting notes, and updated interface previews. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EOGMonitor (GUI)
participant LSL Stream
User->>EOGMonitor (GUI): Start application
EOGMonitor (GUI)->>LSL Stream: Connect and pull EOG samples
loop Real-time processing
EOGMonitor (GUI)->>EOGMonitor (GUI): Filter signal, detect peaks
alt Blink detected
EOGMonitor (GUI)->>EOGMonitor (GUI): Classify as single, double, or triple blink
EOGMonitor (GUI)->>EOGMonitor (GUI): Visualize blink event
end
end
EOGMonitor (GUI)->>User: Display plots and blink detections
sequenceDiagram
participant User
participant MorseCodeEOGSystem
participant LSL Stream
participant GUI (optional)
User->>MorseCodeEOGSystem: Start decoder
MorseCodeEOGSystem->>LSL Stream: Connect and pull EOG samples
loop Real-time processing
MorseCodeEOGSystem->>MorseCodeEOGSystem: Filter and buffer signal
MorseCodeEOGSystem->>MorseCodeEOGSystem: Detect blinks and movements
alt Double blink detected
MorseCodeEOGSystem->>MorseCodeEOGSystem: Translate Morse buffer to character
MorseCodeEOGSystem->>GUI: Update label (if GUI)
else Movement detected
MorseCodeEOGSystem->>MorseCodeEOGSystem: Append dot/dash to Morse buffer
end
end
MorseCodeEOGSystem->>User: Output decoded text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (11)
chordspy/double_triple_blink.py (4)
3-3
: Remove unused importThe
QHBoxLayout
import is not used anywhere in the code.-from PyQt5.QtWidgets import QApplication, QVBoxLayout, QMainWindow, QWidget, QHBoxLayout +from PyQt5.QtWidgets import QApplication, QVBoxLayout, QMainWindow, QWidget
62-65
: Consider more graceful error handling for missing LSL streamsInstead of calling
sys.exit()
directly, consider raising an exception or showing an error dialog to allow the application to handle the error more gracefully.- if not available_streams: - print("No LSL streams found! Exiting...") - sys.exit(0) + if not available_streams: + error_msg = "No LSL streams found!" + print(error_msg) + raise RuntimeError(error_msg)Similarly for the connection failure case:
- if self.inlet is None: - print("Unable to connect to any LSL stream! Exiting...") - sys.exit(0) + if self.inlet is None: + error_msg = "Unable to connect to any LSL stream!" + print(error_msg) + raise RuntimeError(error_msg)Also applies to: 75-78
93-96
: Handle additional sampling rates or use dynamic Y-rangeThe Y-range setting only handles 250Hz and 500Hz sampling rates. Consider handling other rates or using a dynamic approach.
- if self.sampling_rate == 250: - self.eog_plot.setYRange(0, 2**10, padding=0) - elif self.sampling_rate == 500: - self.eog_plot.setYRange(0, 5000, padding=0) + # Use a dynamic approach based on sampling rate + # Higher sampling rates typically have larger ADC ranges + if self.sampling_rate <= 250: + self.eog_plot.setYRange(0, 2**10, padding=0) + elif self.sampling_rate <= 500: + self.eog_plot.setYRange(0, 5000, padding=0) + else: + self.eog_plot.setYRange(0, 10000, padding=0)
248-248
: Consider making minimum peak gap configurableThe
min_peak_gap
is hardcoded to 0.1 seconds. Consider making it a class parameter for flexibility.+ # In __init__ method, add: + self.min_peak_gap = 0.1 # Minimum time gap between peaks in seconds + def detect_peaks(self, signal, threshold): peaks = [] prev_peak_time = None - min_peak_gap = 0.1 # Minimum time gap between two peaks in seconds for i in range(1, len(signal) - 1): if signal[i] > signal[i - 1] and signal[i] > signal[i + 1] and signal[i] > threshold: current_peak_time = i / self.sampling_rate if prev_peak_time is not None: time_gap = current_peak_time - prev_peak_time - if time_gap < min_peak_gap: + if time_gap < self.min_peak_gap: continuechordspy/config/apps.yaml (1)
70-70
: Add newline at end of fileYAML files should end with a newline character for better compatibility with various tools.
icon: "fa-font" color: "rose" script: "morse_decoder" description: "Decode EOG Signals into Alphabets based on Morse Code." category: "EOG" +
README.md (2)
51-51
: Fix markdown syntax for section headerThe syntax
# [!Optional]
is not valid markdown. Consider using a proper heading or note format.-# [!Optional] +## Optional: Running Individual ScriptsOr if you want to keep it as a note:
-# [!Optional] +> [!NOTE] +> **Optional:** If you want to run the individual scripts, follow these steps:
65-69
: Remove duplicate execution policy instructionsThe execution policy instructions are already provided in lines 30-34. This duplication can be removed.
-[!IMPORTANT] -You may get an execution policy error if scripts are restricted. To fix it, run: -```bash -Set-ExecutionPolicy Unrestricted -Scope Process -```chordspy/morse_decoder.py (4)
43-46
: Maintain consistency with error handling across modulesSimilar to
double_triple_blink.py
, consider using exceptions instead ofsys.exit()
for better error handling.- if not available_streams: - print("No LSL streams found! Exiting...") - sys.exit(0) + if not available_streams: + error_msg = "No LSL streams found!" + print(error_msg) + raise RuntimeError(error_msg)And:
- if self.inlet is None: - print("Unable to connect to any LSL stream! Exiting...") - sys.exit(0) + if self.inlet is None: + error_msg = "Unable to connect to any LSL stream!" + print(error_msg) + raise RuntimeError(error_msg)Also applies to: 56-59
291-298
: Simplify nested if statementsThe nested if statements can be combined for better readability.
- if self.movement_samples >= self.MIN_MOVEMENT_SAMPLES: - if detected_state != self.current_state: + if self.movement_samples >= self.MIN_MOVEMENT_SAMPLES and detected_state != self.current_state: self.movement_sequence.append(detected_state) self.current_state = detected_state self.last_movement = detected_state self.cooldown_counter = self.COOLDOWN_SAMPLES self.movement_samples = 0 self.check_movement_completion()
321-325
: Simplify nested if statements for inactivity timeoutThe nested if statements can be combined for better readability.
- if self.morse_buffer and self.last_input_time: - if time.time() - self.last_input_time > self.inactivity_timeout: + if self.morse_buffer and self.last_input_time and time.time() - self.last_input_time > self.inactivity_timeout: print(f"\nBuffer timeout. Clearing morse buffer: {self.morse_buffer}") self.morse_buffer = "" self.last_input_time = None
186-189
: Consider providing feedback for invalid Morse code sequencesCurrently, invalid Morse code sequences are silently discarded. Consider providing user feedback.
else: - # print(f" -> Unknown morse code: {self.morse_buffer}") + if self.morse_buffer: # Only print if buffer is not empty + print(f" -> Unknown morse code: {self.morse_buffer}") self.morse_buffer = "" self.last_input_time = None # Reset last input time after decoding
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(2 hunks)chordspy/config/apps.yaml
(1 hunks)chordspy/double_triple_blink.py
(1 hunks)chordspy/morse_decoder.py
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
chordspy/config/apps.yaml
[error] 70-70: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Ruff (0.12.2)
chordspy/morse_decoder.py
291-292: Use a single if
statement instead of nested if
statements
(SIM102)
321-322: Use a single if
statement instead of nested if
statements
(SIM102)
chordspy/double_triple_blink.py
3-3: PyQt5.QtWidgets.QHBoxLayout
imported but unused
Remove unused import: PyQt5.QtWidgets.QHBoxLayout
(F401)
# Filter only the new data | ||
filtered_eog, self.zi = lfilter(self.b, self.a, self.eog_data, zi=self.zi) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient filtering - filters entire buffer instead of new samples
The comment states "Filter only the new data" but the code filters the entire eog_data
buffer on each update, which is inefficient for a 5-second buffer at high sampling rates.
Consider filtering only the new samples to improve performance:
- # Filter only the new data
- filtered_eog, self.zi = lfilter(self.b, self.a, self.eog_data, zi=self.zi)
+ # Filter the entire buffer for consistent visualization
+ # Note: This is less efficient but ensures consistent filtering across the buffer
+ filtered_eog = lfilter(self.b, self.a, self.eog_data)[0]
Or implement incremental filtering if performance is critical.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In chordspy/double_triple_blink.py around lines 119 to 121, the code filters the
entire eog_data buffer each time, which is inefficient given the large buffer
size. Modify the code to apply the lfilter only to the new incoming samples
instead of the whole buffer, updating the filter state accordingly. This can be
done by slicing the new data portion from eog_data and filtering just that
segment, then updating self.zi with the new filter state to maintain continuity.
self.triple_timer.timeout.connect(lambda: self.double_blink_timeout(filtered_eog)) | ||
self.triple_timer.start(self.double_triple_window) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential stale data in timer callback
The filtered_eog
parameter passed to the lambda function is captured at the time of timer creation, but may be stale when the timer fires 500ms later. This could lead to incorrect marker positions on the plot.
Consider storing the peak indices and retrieving the current filtered data when needed:
- self.triple_timer.timeout.connect(lambda: self.double_blink_timeout(filtered_eog))
+ self.triple_timer.timeout.connect(self.double_blink_timeout)
- def double_blink_timeout(self, filtered_eog):
+ def double_blink_timeout(self):
if self.locked:
return
+ # Use the current filtered data
+ filtered_eog = self.filtered_eog_data if hasattr(self, 'filtered_eog_data') else self.eog_data
# Called if no 3rd blink appears in the window
if len(self.blink_times) >= 2:
Also, consider storing filtered_eog_data
as an instance variable if not already done.
Also applies to: 218-231
🤖 Prompt for AI Agents
In chordspy/double_triple_blink.py around lines 215 to 231, the lambda connected
to the triple_timer.timeout captures filtered_eog at timer creation, which may
be outdated when the timer fires. To fix this, avoid capturing filtered_eog in
the lambda; instead, store the necessary peak indices as instance variables and
retrieve the current filtered_eog data inside the callback function at runtime.
Also, ensure filtered_eog_data is stored as an instance variable if it isn't
already, so the callback can access the latest data when triggered.
if self.filter_state_notch[0] == -1: | ||
filtered, self.filter_state_notch = lfilter(self.notch_b, self.notch_a, [sample], zi=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unusual filter state initialization check
The code checks if filter_state[0] == -1
, but the filter states are initialized to zeros in line 115-116. This condition will never be true.
Either initialize the filter states to -1 or remove the unnecessary checks:
- if self.filter_state_notch[0] == -1:
- filtered, self.filter_state_notch = lfilter(self.notch_b, self.notch_a, [sample], zi=None)
- else:
- filtered, self.filter_state_notch = lfilter(self.notch_b, self.notch_a, [sample], zi=self.filter_state_notch)
+ filtered, self.filter_state_notch = lfilter(self.notch_b, self.notch_a, [sample], zi=self.filter_state_notch)
Apply the same fix for the bandpass filter.
Also applies to: 125-126
🤖 Prompt for AI Agents
In chordspy/morse_decoder.py around lines 120-121 and 125-126, the code checks
if the first element of filter_state_notch and filter_state_bandpass equals -1,
but these states are initialized to zeros earlier, so the condition is never
true. To fix this, either initialize these filter states to -1 at their
declaration or remove the conditional checks entirely since they are redundant.
Apply the same fix consistently for both notch and bandpass filter states.
|
||
for sample in samples: | ||
# Store data for blink detection (channel 1) | ||
self.eog_data[self.current_index] = sample[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider making EOG channel selection configurable
The code hardcodes channel 1 for blink detection and channel 0 for left/right movement. Different EOG setups might use different channel configurations.
Add channel configuration to the __init__
method:
- def __init__(self, gui_label=None, gui_root=None):
+ def __init__(self, gui_label=None, gui_root=None, blink_channel=1, movement_channel=0):
+ self.blink_channel = blink_channel
+ self.movement_channel = movement_channel
Then use these variables:
- self.eog_data[self.current_index] = sample[1]
+ self.eog_data[self.current_index] = sample[self.blink_channel]
- filtered_sample, self.zi = lfilter(self.b, self.a, [sample[1]], zi=self.zi)
+ filtered_sample, self.zi = lfilter(self.b, self.a, [sample[self.blink_channel]], zi=self.zi)
- filtered_sample_lr = sample[0] # Using raw signal for left/right
+ filtered_sample_lr = sample[self.movement_channel] # Using raw signal for left/right
Also applies to: 266-266
🤖 Prompt for AI Agents
In chordspy/morse_decoder.py at lines 259 and 266, the EOG channel indices for
blink detection and left/right movement are hardcoded as 1 and 0 respectively.
To fix this, add parameters for these channel indices to the __init__ method of
the class, allowing them to be set during initialization. Then replace the
hardcoded channel indices in the code with these instance variables to make the
channel selection configurable for different EOG setups.
Summary by CodeRabbit
New Features
Documentation